Enforcement-core hardening: fail-open & policy-widening fixes (#41)#46
Open
davidcrowe wants to merge 1 commit into
Open
Enforcement-core hardening: fail-open & policy-widening fixes (#41)#46davidcrowe wants to merge 1 commit into
davidcrowe wants to merge 1 commit into
Conversation
Security review of the OSS enforcement packages. Each finding is a
reputational/external-consumer correctness issue (prod's coding-agent hook
path uses a private engine), but the repo is the trust artifact.
validatabl-core:
- decision(): a configured inputSchema was skipped when input was undefined
(GET, wrong content-type), so a schema meant to gate a route was a silent
no-op. Now the schema always runs; missing input denies.
- hasScope(): built a RegExp from the raw scope string, so `tool.write`
matched `toolXwrite` (`.` = any char) and metachars threw 500s. Replaced
with an exact whitespace-delimited token match — no regex.
- policy widening: `matches` was unanchored (`search` allowed
`unsafe-search-exec`) — bare patterns are now anchored ^(?:…)$ while an
author's own anchors are honored; `in` used String() coercion (an array
field stringified to "a,b" and slipped past an allow-list) — now strict
string membership; empty-conditions rules matched every request (vacuous
every()) — validatePolicySet() rejects them (loud) and applyPolicies calls
it.
limitabl-core / limitabl:
- AgentGuard was client-opt-out: the guard only ran when the client supplied
a workflow id, so omitting it bypassed runaway protection. preflight() now
denies when a guard is configured but no key resolves. The facade namespaces
the client workflow id under the server-resolved principal so it is not
purely client-controlled. AgentGuard adds a lazy TTL sweep (evict workflows
past maxDurationMs) so id rotation can't grow the map unbounded.
- The facade's module-level singleton engine ignored every config after the
first (a stricter later mount reused the laxer engine). Build one engine per
limitabl() invocation.
identifiabl-core:
- jwtVerify did not require `exp`, so a token minted without it never expired.
Added requiredClaims:["exp"]. The factory now rejects an empty issuer or
audience (empty audience disables audience binding; empty issuer makes the
post-verify issuer check pass for iss:"").
explicabl:
- Audit events read res.locals.gatewayContext, which nothing sets — every
line logged context: undefined. Now reads getGatewayContext() (ALS), with a
captured-reference fallback, and warns once if absent.
- /webhooks/auth0 mounted the handler FACTORY as middleware, so requests hung
forever and the webhook was dead. Call auth0LogsWebhook().
- Only `finish` was hooked, so aborted requests emitted no audit. Also hook
`close`, with a fired-once guard so exactly one event emits.
- Added @gatewaystack/request-context dependency + tsconfig reference.
Repo hygiene (unblocks the above): removed 16 stale compiled .js/.d.ts
artifacts committed inside src/ dirs. These were already gitignored but
tracked, and several (validatabl-core/src/scopes.js, explicabl/src/index.js,
limitabl/src/index.js) contained the exact pre-fix code this PR removes —
they shadowed the source on .js imports and contradicted the fixes.
Tests: 30 new across the five packages — decision denies on absent input;
hasScope("a.b") ∌ "aXb"; empty-conditions rejected; matches anchored; in
strict; JWT round-trips reject alg:none/expired/missing-exp/wrong-aud + factory
guards; two limitabl mounts enforce different limits; agent guard denies
without a key + TTL evicts; client-abort emits exactly one audit event; webhook
route responds. Full suite 189 green; build clean.
Versions intentionally NOT bumped here — publish/version is David's step, and
explicabl also has a parallel bump in #37; coordinate at publish.
This was referenced Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #41. Security-review hardening of the OSS enforcement packages. These are reputational/external-consumer correctness (prod's coding-agent hook path uses a private engine), but the repo is the trust artifact.
validatabl-core
decision.ts): a configuredinputSchemawas skipped wheninputwasundefined(GET, wrong content-type), so a schema meant to gate a route was a silent no-op. Now the schema always runs; missing input denies.hasScoperegex injection (scopes.ts): built aRegExpfrom the raw scope, sotool.writematchedtoolXwriteand metachars threw 500s. Now an exact whitespace-delimited token match — no regex.policy.ts):matcheswas unanchored (searchallowedunsafe-search-exec) → bare patterns anchored^(?:…)$, author anchors honored;inusedString()coercion (array field →"a,b"slipped past an allow-list) → strict string membership; empty-conditions rules matched every request →validatePolicySet()rejects them loudly andapplyPoliciescalls it.limitabl-core / limitabl
preflight()now denies when a guard is configured but no key resolves. The facade namespaces the client workflow id under the server-resolved principal (sub/tenant/ip) so it is not purely client-controlled, andAgentGuardgains a lazy TTL sweep (evict workflows pastmaxDurationMs) so id rotation can't grow the map unbounded.limitabl()invocation.identifiabl-core
expnot required:jwtVerifyaccepted a token minted withoutexp— valid forever. AddedrequiredClaims:["exp"]. The factory now rejects an empty issuer or audience (empty audience disables audience binding; empty issuer makes the post-verify issuer check pass foriss:"").explicabl
res.locals.gatewayContext, which nothing sets — every line loggedcontext: undefined. Now readsgetGatewayContext()(ALS) with a captured-reference fallback, and warns once if absent./webhooks/auth0mounted the handler factory as middleware → requests hung forever. Now callsauth0LogsWebhook().finishwas hooked. Also hookclose, with a fired-once guard so exactly one event emits.@gatewaystack/request-contextdependency + tsconfig project reference.Repo hygiene (unblocks the above)
Removed 16 stale compiled
.js/.d.tsartifacts committed insidesrc/dirs. They were already gitignored but tracked, and several —validatabl-core/src/scopes.js,explicabl/src/index.js,limitabl/src/index.js— contained the exact pre-fix code this PR removes, shadowing the source on.jsimports (this actually broke the first run of the newhasScopetest until removed). Overlaps the #42 hygiene theme; done here because it directly contradicts these fixes.Tests (30 new, all real behavior)
decisiondenies on absent input ·hasScope("a.b") ∌ "aXb"· empty-conditions rejected ·matchesanchored ·instrict · JWT round-trips (real jose + local JWKS) rejectalg:none/expired/missing-exp/wrong-aud + factory guards · twolimitablmounts enforce different limits · agent guard denies without a key + TTL evicts · client-abort emits exactly one audit event · webhook route responds (no hang).Full suite 189 green ·
npm run buildclean.Notes
explicablalso has a parallel bump in fix(explicabl): drop undeclared node-fetch import; use global fetch #37 — coordinate the final numbers at publish (packages needing republish:validatabl-core,limitabl-core,limitabl,identifiabl-core,explicabl).Not merged/published — that's yours.